Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor Python module_utils locator #70610

Merged
merged 17 commits into from Jul 17, 2020
Merged

refactor Python module_utils locator #70610

merged 17 commits into from Jul 17, 2020

Conversation

nitzmahone
Copy link
Member

SUMMARY
  • no longer recursive
  • embed special-case module code internally
  • share common code between collections/not cases
  • fixes Broken module_utils imports fail horribly #70134
  • properly support subpackage redirection
  • adds support for FQCN redirect targets used by migration (expands to FQ Python name)
  • add tests
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

module_common

ADDITIONAL INFORMATION

@nitzmahone
refactor Python module_utils locator
6e47c56 
* no longer recursive
* embed special-case module code internally
* share common code between collections/not cases
* fixes ansible#70134
* properly support subpackage redirection
* adds support for FQCN redirect targets used by migration (expands to FQ Python name)
* add tests
@ansibot ansibot added affects_2.11 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. new_plugin This PR includes a new plugin. runtime ansible_builtin_runtime.yml support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 13, 2020
@samdoran samdoran added the ci_verified Changes made in this PR are causing tests to fail. label Jul 13, 2020
@ansibot
Copy link
Contributor

ansibot commented Jul 13, 2020

The test ansible-test sanity --test pylint [explain] failed with 9 errors:

lib/ansible/executor/module_common.py:652:12: unreachable: Unreachable code
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_collection_redirected_mu.py:9:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_collection_redirected_mu.py:10:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing.py:8:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing.py:14:4: unreachable: Unreachable code
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_collection.py:8:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_collection.py:14:4: unreachable: Unreachable code
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_module.py:8:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_module.py:14:4: unreachable: Unreachable code

The test ansible-test sanity --test pep8 [explain] failed with 5 errors:

lib/ansible/executor/module_common.py:641:5: E303: too many blank lines (3)
lib/ansible/executor/module_common.py:740:5: E303: too many blank lines (2)
lib/ansible/executor/module_common.py:789:161: E501: line too long (164 > 160 characters)
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_collection_redirected_mu.py:12:1: E302: expected 2 blank lines, found 1
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_module_import_from.py:10:1: E265: block comment should start with '# '

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jul 13, 2020
jborean93
jborean93 reviewed Jul 13, 2020
View reviewed changes
lib/ansible/config/ansible_builtin_runtime.yml Outdated Show resolved Hide resolved
lib/ansible/executor/module_common.py


def recursive_finder(name, module_fqn, data, py_module_names, py_module_cache, zf):
def recursive_finder(name, module_fqn, module_data, zf):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we concerned that changing the signature here could break an public interfaces?

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jul 14, 2020
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 16, 2020
@ansibot
Copy link
Contributor

ansibot commented Jul 16, 2020

The test ansible-test sanity --test pylint [explain] failed with 10 errors:

lib/ansible/executor/module_common.py:685:23: missing-format-argument-key: Missing keyword argument '0)' for format string
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/module_utils/subpkg_with_init/__init__.py:6:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_collection_redirected_mu.py:9:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_collection_redirected_mu.py:10:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing.py:8:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing.py:14:4: unreachable: Unreachable code
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_collection.py:8:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_collection.py:14:4: unreachable: Unreachable code
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_module.py:8:0: relative-beyond-top-level: Attempted relative import beyond top-level package
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_mu_missing_redirect_module.py:14:4: unreachable: Unreachable code

The test ansible-test sanity --test pep8 [explain] failed with 8 errors:

lib/ansible/executor/module_common.py:666:161: E501: line too long (172 > 160 characters)
lib/ansible/executor/module_common.py:779:5: E303: too many blank lines (2)
lib/ansible/executor/module_common.py:828:161: E501: line too long (164 > 160 characters)
lib/ansible/executor/module_common.py:934:161: E501: line too long (162 > 160 characters)
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/module_utils/subpkg_with_init/__init__.py:8:1: E302: expected 2 blank lines, found 1
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_collection_redirected_mu.py:12:1: E302: expected 2 blank lines, found 1
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_module_import_from.py:10:1: E265: block comment should start with '# '
test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/uses_leaf_mu_module_import_from.py:15:1: E302: expected 2 blank lines, found 1

click here for bot help

@ansibot ansibot removed the core_review In order to be merged, this PR must follow the core review workflow. label Jul 16, 2020
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 16, 2020
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 16, 2020
@nitzmahone
pylint
9c7e3ae 
ci_complete
@nitzmahone nitzmahone marked this pull request as ready for review July 16, 2020 23:59
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 17, 2020
@nitzmahone nitzmahone merged commit c616e54 into ansible:devel Jul 17, 2020
nitzmahone added a commit to nitzmahone/ansible that referenced this pull request Jul 17, 2020
@nitzmahone
refactor Python module_utils locator (ansible#70610)
1f34012 
* refactor Python module_utils locator

* no longer recursive
* embed special-case module code internally
* share common code between collections/not cases
* fixes ansible#70134
* properly support subpackage redirection
* adds support for FQCN redirect targets used by migration (expands to FQ Python name)
* add tests

* add changelog

(cherry picked from commit c616e54)
@nitzmahone nitzmahone mentioned this pull request Jul 17, 2020
Merged
nitzmahone added a commit that referenced this pull request Jul 17, 2020
@nitzmahone
refactor Python module_utils locator (#70610) (#70711)
1e03b54 
* refactor Python module_utils locator

* no longer recursive
* embed special-case module code internally
* share common code between collections/not cases
* fixes #70134
* properly support subpackage redirection
* adds support for FQCN redirect targets used by migration (expands to FQ Python name)
* add tests

* add changelog

(cherry picked from commit c616e54)
felixfontein added a commit to felixfontein/ansible that referenced this pull request Jul 18, 2020
@felixfontein
Followup to ansible#70610: fix redirects to wrong collections
1c27479
@felixfontein felixfontein mentioned this pull request Jul 18, 2020
Merged
relrod pushed a commit that referenced this pull request Jul 20, 2020
@felixfontein
Followup to #70610: fix ansible_builtin_runtime.yml redirects to wron…
cf21717 
…g collections (#70725)

* Followup to #70610: fix redirects to wrong collections

* gcp module_utils is in community.general.

* dellos10_* modules are in dellemc_networking.os10, not dellemc.os10.

* intersight_facts was renamed to intersight_info in cisco.intersight in version 1.0.7.

* Fix more dellos10 redirects.
felixfontein added a commit to felixfontein/ansible that referenced this pull request Jul 20, 2020
@felixfontein
Followup to ansible#70610: fix ansible_builtin_runtime.yml redirects …
caa9a2e 
…to wrong collections (ansible#70725)

* Followup to ansible#70610: fix redirects to wrong collections

* gcp module_utils is in community.general.

* dellos10_* modules are in dellemc_networking.os10, not dellemc.os10.

* intersight_facts was renamed to intersight_info in cisco.intersight in version 1.0.7.

* Fix more dellos10 redirects.

(cherry picked from commit cf21717)
gundalow pushed a commit that referenced this pull request Jul 20, 2020
@felixfontein
Followup to #70610: fix ansible_builtin_runtime.yml redirects to wron…
a5b46b6 
…g collections (#70725) (#70743)

* Followup to #70610: fix redirects to wrong collections

* gcp module_utils is in community.general.

* dellos10_* modules are in dellemc_networking.os10, not dellemc.os10.

* intersight_facts was renamed to intersight_info in cisco.intersight in version 1.0.7.

* Fix more dellos10 redirects.

(cherry picked from commit cf21717)
@ansible ansible locked and limited conversation to collaborators Aug 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jborean93 jborean93 jborean93 left review comments

Assignees
No one assigned
Labels
affects_2.11 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. new_plugin This PR includes a new plugin. runtime ansible_builtin_runtime.yml support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Broken module_utils imports fail horribly
4 participants
@nitzmahone @ansibot @jborean93 @samdoran